ADFA-6381: Build one Kotlin library module per jar by hal-eisen-adfa · Pull Request #2108 · appdevforall/CodeOnTheGo

hal-eisen-adfa · 2026-10-06T18:12:37Z

The Kotlin LSP built a separate KtLibraryModule for every shared jar in every module, and each copy materializes the jar's full file list for its search scope. This is behind about 65% of GlitchTip OOM events (219 of 336), mostly on large multi-module projects.

Cause

In collectKtModules, bootClassPaths was a lazy Sequence whose pipeline called addLibrary. Every source module re-ran it, so each one got a fresh android.jar module from every Android module: modules x Android-modules copies. addLibrary also never checked jarToModMap, so a jar repeated across compile classpaths built a new module every time.

Fix

addLibrary returns the existing module for a path (getOrPut).
The boot classpath is collected once into a distinct() List.
KtSourceModule.Builder and KtLibraryModule.Builder keep dependencies in a LinkedHashSet, so a jar on both the boot and compile classpath is one dependency, not the same instance twice (O(1) per add, order kept).

Verification

New CollectKtModulesTest (3 Android modules sharing one android.jar): fails on the old code (3 copies per module, expected 1), passes with the fix.
CollectKtModulesTest, boot + compile classpath: fails without the builder change (size 2, expected 1).
CollectKtModulesTest, compile-classpath jar shared by two modules resolves to one instance. This pins existing behavior: the per-path libraryDependencies map already shared it before this PR.
:lsp:kotlin:testV8DebugUnitTest: 50 classes, 532 tests, 0 failures.
Emulator (arm64), the same 3-module Kotlin project (:app + 2 library modules), heap dump after project init:

	Before	After
`KtLibraryModule` instances	56	11
Materialized library search scopes	56	11
`KtSourceModule` instances	3	3
Java heap (`dumpsys meminfo`)	118.3 MB	104.6 MB

The "before" build was an older stage build, so the heap figure is indicative; the instance counts come straight from this code path. Copies grow with modules x Android modules, so the saving is much larger on projects like TalkBack or media3.

On the fixed build, android.* types (Context, TextView, android.widget.TextView::class) resolve with no diagnostics in both library modules.

Siblings checked: collectKtModules is the only production caller of buildKtLibraryModule; libraryDependencies now shares modules through the same getOrPut.

No UI change, so no font-scale check.

Commits: the fix and test; a standalone Spotless reformat of WorkspaceExts.kt; a standalone Spotless reformat of the two module builders; the builder dedupe and its tests.

collectKtModules held the boot classpath as a lazy Sequence that called addLibrary, so every source module re-ran it and got a fresh KtLibraryModule for android.jar from each Android module: modules x Android-modules copies. Each copy materializes the jar's full file list for its search scope. GlitchTip breadcrumbs show the same android.jar scope built 55-71 times on large multi-module projects, behind 219 of 336 OOM events. addLibrary now reuses the module for a path, and the boot classpath is a distinct List, so each jar maps to exactly one module.

claude

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

coderabbitai · 2026-10-06T18:14:49Z

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Essentials
Run ID: 6e1525ee-c48c-46d7-adaa-96f6583d4506

📥 Commits

Reviewing files that changed from the base of the PR and between bbb4678 and 555035d.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Essentials
Run ID: 7ac6c74d-0dd5-496b-89d3-6b2048046f95

📥 Commits

Reviewing files that changed from the base of the PR and between a1fbbac and bbb4678.

📒 Files selected for processing (3)

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtLibraryModule.kt
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtSourceModule.kt
lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/CollectKtModulesTest.kt

Included review availability: This review used your included allowance. 4 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

📝 Summary

Workspace.collectKtModules now reuses KtLibraryModule instances by jar path. Boot classpaths are collected as a distinct list before being added to source modules.
Added CollectKtModulesTest to verify that three Android subprojects sharing one android.jar use the same library module instance.
Reformatted WorkspaceExts.kt in a separate commit; the change is reported as having no functional effect.
The reported tests passed: :lsp:kotlin:testV8DebugUnitTest ran 50 classes with no failures. The reported emulator comparison showed fewer library modules and materialized library search scopes, while source module count remained unchanged. The heap comparison is indicative because it used an older stage build.
Risk: the reported heap comparison is not a controlled before-and-after measurement. No current review findings were supplied, so review severity counts are unavailable.

Walkthrough

collectKtModules caches library modules by jar path across boot and compile classpaths. Module builders deduplicate dependencies while preserving insertion order. Tests cover shared jars across Android and Java modules.

Changes

Shared library modules

Layer / File(s)	Summary
Deduplicate module dependencies `lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtLibraryModule.kt`, `lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtSourceModule.kt`	The library and source module builders store dependencies in insertion-ordered sets. Other module construction and search-scope behavior remains unchanged.
Cache library modules and verify reuse `lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/WorkspaceExts.kt`, `lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/CollectKtModulesTest.kt`	`collectKtModules` caches library modules by jar path and collects Android boot-classpath paths before creating library modules. Tests check shared instances across Android and Java modules, including when a jar appears on both classpaths.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: jatezzz

Merge Risk: ⚪ Minimal · up to bbb46

The change is ready to merge after normal checks; no outstanding behavior issue was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: reusing one Kotlin library module per jar.
Description check	✅ Passed	The description explains the duplicate library-module problem, the fix, and its verification. It is directly related to the changeset.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit checks each jar in line
One shared module keeps its place
Boot and compile paths align
Builders skip a second trace
Tests confirm the links are fine}

_{Comment @coderabbitai help to get the list of available commands.}

Now that addLibrary returns one shared KtLibraryModule per jar, a jar on both the boot classpath and a module's compile classpath was added to that module's dependencies twice as the same instance. Both module builders now keep dependencies in a LinkedHashSet: O(1) dedupe, order preserved. Tests: compile-classpath jars shared across modules resolve to one instance; a jar on both classpaths is one dependency (fails without the builder change: size 2).

hal-eisen-adfa added 2 commits October 6, 2026 09:24

style: spotless reformat, no functional change

a1fbbac

claude Bot reviewed Oct 6, 2026

View reviewed changes

hal-eisen-adfa requested a review from a team October 6, 2026 19:19

jatezzz reviewed Oct 6, 2026

View reviewed changes

Comment thread lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/WorkspaceExts.kt

jatezzz reviewed Oct 6, 2026

View reviewed changes

Comment thread lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/CollectKtModulesTest.kt

hal-eisen-adfa added 2 commits October 6, 2026 15:12

style: spotless reformat, no functional change

c63f304

jatezzz approved these changes Oct 6, 2026

View reviewed changes

Merge branch 'stage' into fix/ADFA-6381-kt-library-module-dedupe

555035d

hal-eisen-adfa merged commit 2d4ab23 into stage Oct 6, 2026
5 checks passed

hal-eisen-adfa deleted the fix/ADFA-6381-kt-library-module-dedupe branch October 6, 2026 23:31

Navigation Menu

Repository navigation

ADFA-6381: Build one Kotlin library module per jar - #2108

ADFA-6381: Build one Kotlin library module per jar#2108
hal-eisen-adfa merged 5 commits into
stagefrom
fix/ADFA-6381-kt-library-module-dedupe

hal-eisen-adfa commented Oct 6, 2026 •

edited by github-actions Bot

Loading

Uh oh!

claude Bot left a comment

Uh oh!

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

hal-eisen-adfa commented Oct 6, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Cause

Fix

Verification

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

coderabbitai Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hal-eisen-adfa commented Oct 6, 2026 •

edited by github-actions Bot

Loading

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading